SF-3155 Fix incorrect draft source language codes #2963
Conversation
pmachapman
added
the
will require testing
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2963 +/- ##
=======================================
Coverage 82.02% 82.02%
=======================================
Files 544 544
Lines 31685 31689 +4
Branches 5152 5149 -3
=======================================
+ Hits 25989 25993 +4
Misses 4927 4927
Partials 769 769 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pmachapman)
src/SIL.XForge.Scripture/Services/ParatextService.cs line 1851 at r1 (raw file):
{ using ScrText scrText = GetScrText(userSecret, ptProjectId); return GetWritingSystem(scrText.Settings.LanguageID.Id);
Is this where the problem is occurring? Somehow the Settings.LanguageId is the better way to calculate the WritingSystem?
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @RaymondLuong3)
src/SIL.XForge.Scripture/Services/ParatextService.cs line 1851 at r1 (raw file):
Previously, RaymondLuong3 (Raymond Luong) wrote…
Is this where the problem is occurring? Somehow the Settings.LanguageId is the better way to calculate the WritingSystem?
Yes - Settings.LanguageID.Id retrieves the language directly from the Settings XML (and so compatible with Paratext 8 resources or projects), while Language.Id uses the language defintion file.
There was a problem hiding this comment.
Reviewable status:
complete! all files reviewed, all discussions resolved (waiting on @pmachapman)
|
@pmachapman I'm pretty sure based on hearing a bit of conversation that this is a bugfix, but "Retrieve the language code from Settings" isn't actually giving any information about the nature of this change. Presumably you were getting it from somewhere else in the past, and it was unreliable, so now you're getting it from a more accurate location? Our guidelines for commit messages (and PR titles) state:
In this case I think it should probably say something like "Fix incorrect draft source language codes," and then the commit/PR body could say "get them from settings instead" (though I still don't know how that makes a difference -- maybe it's immaterial). |
pmachapman
changed the title
There was a problem hiding this comment.
@Nateowami Thanks for catching that. I was thinking like a programmer :-/
Reviewable status:
pmachapman
force-pushed
the
fix/SF-3155
branch
from
4f19310 to
95f63cc
Compare
pmachapman
added
ready to test
and removed
will require testing
Nateowami
added
testing complete
Nateowami
force-pushed
the
fix/SF-3155
branch
from
95f63cc to
3db329c
Compare
This PR fixes the incorrect language codes for resources created in Paratext 8 by retrieving the language code from the Settings XML (in this case an SSF file) instead of from the LDML file which will usually default to English in these cases.
This change is